should we update physical column name as well for better readability in the query output? https://github.com/apache/arrow-datafusion/blob/master/datafusion/src/physical_plan/planner.rs#L137-L139

should we update physical column name as well for better readability in the query output? https://github.com/apache/arrow-datafusion/blob/master/datafusion/src/physical_plan/planner.rs#L137-L139

i think this is more of the strategy on how we name unnamed columns in the schema. for window functions without an alias, it'll be created with a unique name, given the function type, arg name, partition by clause, order by clause, and window frames. any of these are equivalent then the columns can be thought of the same and thus deduplicated in the projection phase, otherwise they will be planned separately.

if any such columns are having a name conflict in the projection phase the schema validation logic will fail.

Postgres is totally okay with dup names in the projection phase:

[postgres] # select max(c1) over (), max(c1) over (), max(c1) over (partition by c1) from test limit 3;
 max | max | max
-----+-----+-----
 e   | e   | a
 e   | e   | a
 e   | e   | a
(3 rows)

if we are okay with dup names then i guess just keeping the function name would be enough

We do allow query outputs (i.e. record batch schema and physical plan schema) to contain columns with same names. This is quite common in join queries where more than one relations contain the same column name.

My previous comment was more around the inconsistency on how window function names are generated in logical plane v.s. physical plane. For example, in physical plane, we don't take partition by clause, order by clause, and window frames into account when creating the column name. If we don't want to maintain this consistency, I think we should update this invariant instead: https://github.com/apache/arrow-datafusion/blob/master/docs/specification/invariants.md#the-physical-schema-is-invariant-under-planning.

https://github.com/apache/arrow-datafusion/blob/master/docs/specification/output-field-name-semantic.md contains some rules on how we want to generate field names for physical schema. We don't have any rules for window functions at the moment, so maybe worth formalize it in that document as well.

We do allow query outputs (i.e. record batch schema and physical plan schema) to contain columns with same names. This is quite common in join queries where more than one relations contain the same column name.

My previous comment was more around the inconsistency on how window function names are generated in logical plane v.s. physical plane. For example, in physical plane, we don't take partition by clause, order by clause, and window frames into account when creating the column name. If we don't want to maintain this consistency, I think we should update this invariant instead: https://github.com/apache/arrow-datafusion/blob/master/docs/specification/invariants.md#the-physical-schema-is-invariant-under-planning.

https://github.com/apache/arrow-datafusion/blob/master/docs/specification/output-field-name-semantic.md contains some rules on how we want to generate field names for physical schema. We don't have any rules for window functions at the moment, so maybe worth formalize it in that document as well.

Thanks for the advice let me address in another pull request